Skip to content

feat(linear): v1.1 polish — pre-container feedback, state-on-start, sweep, nits#87

Open
isadeks wants to merge 5 commits into
aws-samples:mainfrom
isadeks:feat/linear-processor-feedback
Open

feat(linear): v1.1 polish — pre-container feedback, state-on-start, sweep, nits#87
isadeks wants to merge 5 commits into
aws-samples:mainfrom
isadeks:feat/linear-processor-feedback

Conversation

@isadeks
Copy link
Copy Markdown
Contributor

@isadeks isadeks commented May 13, 2026

Summary

Wraps the v1.1 polish theme for the Linear integration, addressing the silent-drop gaps and Alain's #63 review recommendations in a single PR. Six rejection paths now produce user-visible feedback; the issue state transitions on agent-start (not just at PR-open); a stale-reaction sweep keeps re-runs clean; and four small Alain-review nits are addressed.

What changed

1. Pre-container failure feedback (5 paths in processor + 1 in orchestrator)

Linear-triggered tasks that fail before the agent container starts now produce a Linear comment + ❌ reaction within ~20s of label-apply. Previously, six distinct rejection points all silently dropped:

# Trigger Layer Message
1 Issue has no projectId processor "isn't in a project — ABCA needs a Linear project to route tasks to a repo"
2 Project not onboarded / removed processor "isn't onboarded; admin can run bgagent linear onboard-project"
3 Webhook missing organization or actor processor diagnostic ("report to your ABCA admin")
4 Linear actor has no linked platform user processor "v1 only the API-token owner can submit; multi-user OAuth on v3 roadmap"
5 createTaskCore returns non-201 processor branched on status: guardrail/validation surfaces error string; 503 prompts retry; other 4xx/5xx generic
6 User concurrency cap rejection orchestrator "concurrency limit; wait for one to finish, then re-apply the label"

All six call sites use a shared helper cdk/src/handlers/shared/linear-feedback.tsreportIssueFailure(secretArn, issueId, message) posts the comment and reaction in parallel via Promise.allSettled. Reuses the existing 5-minute getLinearSecret cache.

2. State transition at agent-start

Mirrors the existing In Review transition that fires at PR-open. The prompt addendum's step 1 now instructs the agent to transition Backlog/TodoIn Progress before doing repo work, so Linear's state column reflects "being worked" during the minutes between label-apply and PR-open. Falls back to Todo if In Progress doesn't exist; skips if the issue is already in In Progress or any later state. Doesn't loop on list_issue_statuses.

3. Stale-reaction sweep

Re-runs (label removed and re-applied; or pre-container ❌ followed by a successful retry) used to leave prior 👀/✅/❌ accumulated next to the new run's reaction. Added _sweep_stale_reactions(issue_id) to agent/src/linear_reactions.py:

  • Architecture: 👀-first, sweep-async — the user-visible 👀 fires first (~150ms typical). The sweep runs on a daemon thread after the 👀 lands, so cold-network latency (Linear's first call from a fresh container can be ~5s) doesn't gate the user-visible signal or block the agent pipeline.
  • Filters by viewer + emoji — only deletes the bot's own 👀/✅/❌, never touches reactions a human added even if they happen to use the same emoji.
  • Excludes the just-posted 👀 from its own sweep (passes exclude_id=rid).
  • Best-effort — failure leaves the visual-duplication bug we set out to fix, but the pipeline proceeds unaffected.

Smoke-tested on backgroundagent-dev: react_task_started total = 156ms; sweep done in background at +303ms; agent started at +3ms after react_task_started exit.

4. Alain #63 review nits (4)

  • Auth circuit breaker in linear_reactions.py — track consecutive 401/403s; after 3 strikes, ERROR once and short-circuit until container restart. Replaces the prior behaviour where revoked tokens flooded CloudWatch with WARNs forever.
  • Explicit linear_eyes_reaction_id: str | None = None init in pipeline.py instead of locals().get(...) in the crash handler.
  • Narrow except Exception in resolve_linear_api_token to (BotoCoreError, ClientError). Switch print()shell.log("WARN", ...).
  • Admin-assisted fallback docs for bgagent linear setup auto-link failures. Replaces misleading "run bgagent linear link <code>" suggestion (the command is non-functional in v1) with explicit DynamoDB put-item steps and a clear v3-OAuth note.

Plumbing

  • linear-integration.ts — wires LINEAR_API_TOKEN_SECRET_ARN into the webhook processor + grants read on LinearIntegration.apiTokenSecret.
  • agent.ts — same wiring for orchestrator.fn (declared after LinearIntegration so done in the stack rather than as a prop).
  • Both grants verified at synth: LinearIntegrationWebhookProcessorFn and TaskOrchestratorOrchestratorFn both carry the env var and IAM policy.

Test plan

Unit tests — 1240 baseline → 1268 CDK, 526 → 532 agent, 196 CLI, all green:

  • cdk/test/handlers/shared/linear-feedback.test.ts (new, 13 tests) — mutation shape, auth header, error swallowing in 4 distinct failure modes, Promise.allSettled partial-success semantics.
  • cdk/test/handlers/linear-webhook-processor.test.ts — extended with a user-visible feedback describe block, 10 new tests: one assertion per rejection path + happy-path-doesn't-fire + filter-rejection-doesn't-fire (latter is intentional UX).
  • cdk/test/handlers/orchestrate-task-feedback.test.ts (new, 5 tests) — covers notifyLinearOnConcurrencyCap with withDurableExecution mocked.
  • agent/tests/test_linear_reactions.py — extended with 5 sweep tests covering scoping rules (viewer-owned bgagent emojis only; preserves human reactions even with same emoji; preserves bot reactions of other emojis; sweep failure doesn't block 👀; viewer-id cached across calls).

Lint + synth + agent quality: clean.

Smoke tested on backgroundagent-dev:

  • Path 2 (project not onboarded) ✅
  • Path 5a (guardrail block) ✅ — ❌ + comment within ~20s of label
  • 👀-fast architecture ✅ — eyes-visible at +156ms confirmed via runtime container logs

Smoke tests not run for paths 3 (defensive, no natural reproducer) and 5b (503, no natural reproducer) — covered by unit tests. Path 6 (concurrency cap) requires 4 simultaneous tasks to reproduce; covered by unit tests.

Reviewer notes

  • No linear_*_msg_ts fields on TaskRecord — all per-issue state stays in Linear, accessed at runtime.
  • No new stream consumers — reactions/comments use direct GraphQL from agent + Lambda; doesn't touch the FanOutConsumer / TaskEventsTable 2-reader-per-shard limit (closed by feat(fanout): migrate SlackNotifyFn to FanOutConsumer subscriber (#64) #79 anyway).
  • Sweep is daemon-threaded — never blocks the agent pipeline. If the container terminates before the sweep finishes, the worst case is the visual-duplication bug we set out to fix (status quo from before this PR).
  • Auth circuit breaker is per-container — resets on container restart, doesn't persist to DynamoDB. Right tradeoff for a 5-15 minute task; revisit if container lifetimes grow much longer.

Closes the silent-drop UX gap that appeared whenever a Linear-triggered task
was rejected before the agent container started — the user would apply the
trigger label, see nothing happen, and have no way to know why. Reactions
and progress comments are emitted by the agent container; nothing fired
until that point, so all upstream rejections were invisible on the Linear
side.

This commit wires a best-effort GraphQL feedback path covering all six
distinct rejection points:

In `linear-webhook-processor.ts` (pre-`createTaskCore`):
  1. Issue has no projectId → "isn't in a project" comment
  2. Project not onboarded / removed → "isn't onboarded; admin can run
     `bgagent linear onboard-project`" comment
  3. Webhook missing organization or actor → diagnostic comment
  4. Linear actor has no linked platform user → "v1 only the API-token
     owner can submit; multi-user OAuth is on the v3 roadmap" comment
  5. `createTaskCore` returns non-201 → message branched on status:
     guardrail/validation block surfaces the user-facing error string;
     503 prompts the user to re-apply the label; other 4xx/5xx falls
     through to a generic message.

In `orchestrate-task.ts` (post-201, in admission control):
  6. User concurrency cap rejection → "concurrency limit; wait for one
     to finish, then re-apply the label" comment.

All five processor paths and the orchestrator path call a shared helper,
`reportIssueFailure(secretArn, issueId, message)`, that runs the comment
and ❌ reaction in parallel via `Promise.allSettled`. The helper:

  - Reuses the existing 5-minute `getLinearSecret` cache from
    `linear-verify.ts` (no extra Secrets Manager hits on warm Lambdas).
  - Swallows network, auth, and GraphQL errors with WARN logs — Linear
    feedback is advisory and must never gate the rejection path.
  - Posts to Linear's hosted GraphQL endpoint; mutation shapes match
    `agent/src/linear_reactions.py` (`commentCreate`, `reactionCreate`).

CDK plumbing:

  - `linear-integration.ts` — wires `LINEAR_API_TOKEN_SECRET_ARN` into
    the webhook processor and grants read on the existing
    `LinearIntegration.apiTokenSecret`.
  - `agent.ts` — grants the same secret to `orchestrator.fn` and
    populates the env var. The grant is unconditional; the orchestrator
    only invokes the helper when `task.channel_source === 'linear'`.

The non-Linear case is a hard no-op at the call site — `notifyLinear-
OnConcurrencyCap` early-returns on `channel_source !== 'linear'`, and the
processor only handles Linear payloads. Slack/API/webhook tasks are
unaffected.

Tests (28 new; 1240 → 1268, all green):

  - `cdk/test/handlers/shared/linear-feedback.test.ts` (13 tests):
    mutation shape, auth header, error swallowing in 4 distinct failure
    modes (secret-resolution null, non-2xx, GraphQL `errors`, network
    throw), `Promise.allSettled` partial-success semantics.
  - `cdk/test/handlers/linear-webhook-processor.test.ts` (10 new tests
    in a `user-visible feedback` describe block): one assertion per
    rejection path + happy-path-doesn't-fire + filter-rejection-doesn't-
    fire (the latter is intentional UX — the processor sees many events
    that aren't tasks, and dropping a comment on each would be noisy).
  - `cdk/test/handlers/orchestrate-task-feedback.test.ts` (5 tests):
    new file; covers `notifyLinearOnConcurrencyCap` directly with
    `withDurableExecution` mocked. Asserts the linear path fires; the
    api/webhook/slack paths no-op; missing metadata, missing env, and
    undefined `channel_metadata` all no-op cleanly.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
@isadeks isadeks requested a review from a team as a code owner May 13, 2026 22:35
@isadeks isadeks marked this pull request as draft May 13, 2026 22:47
 nits

Wraps the v1.1 polish theme from PR aws-samples#87. Five small additions, all
agent-side or docs:

State-on-start (the user-visible one):
  - prompt_builder._channel_prompt_addendum now instructs the agent to
    transition the originating Linear issue to `In Progress` (or `Todo`
    fallback) at agent-start, mirroring the existing `In Review` chain
    fired at PR-open. Closes the gap where the issue stayed at `Backlog`
    during real agent work — only the 👀 reaction and "🤖 Starting"
    comment signaled progress, while humans-using-Linear expect the state
    column to reflect "being worked." Skips if the issue is already in
    `In Progress` or any later state; doesn't loop on list_issue_statuses.

Alain aws-samples#63 review nits (4 small surgical changes):
  - linear_reactions.py: auth-failure circuit breaker. Track consecutive
    401/403s; after 3 strikes, log ERROR once and short-circuit all later
    _graphql calls (return None) until the container restarts. Resets on
    any 2xx response. Replaces the prior behaviour where revoked tokens
    flooded CloudWatch with WARNs and wasted Linear API quota indefinitely.
  - pipeline.py: declare `linear_eyes_reaction_id: str | None = None`
    explicitly before the try block instead of relying on
    `locals().get("linear_eyes_reaction_id")` in the crash handler.
    Functionally identical; survives refactors and reads cleanly.
  - config.py::resolve_linear_api_token: narrow `except Exception` to
    `(BotoCoreError, ClientError)` from botocore.exceptions. Switch
    `print()` to `shell.log("WARN", ...)` so warnings join the structured
    log stream the rest of the agent uses.
  - LINEAR_SETUP_GUIDE.md + cli/src/commands/linear.ts: stop telling
    users to run `bgagent linear link <code>` when auto-link fails — the
    code generator is a v3 feature that doesn't ship in v1, so the
    suggestion was misleading. Replaced with explicit admin-assisted
    fallback (DynamoDB put-item with steps to find workspaceId, viewerId,
    Cognito sub) and a clear "this command exists but is non-functional
    in v1" note.

Tests: 532 agent + 1268 cdk + 196 cli, all green. Deployed to
backgroundagent-dev. Smoke-tested 👀-on-start (156ms, agent unblocked)
in the prior commit; state-on-start smoke is the next manual step.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
@isadeks isadeks changed the title feat(linear): comment + react on pre-container task-creation failures feat(linear): v1.1 polish — pre-container feedback, state-on-start, sweep, nits May 14, 2026
Whitespace-only changes flagged by CI's self-mutation guard. No behaviour
change.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
@isadeks isadeks marked this pull request as ready for review May 14, 2026 21:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant